Conversation
derekperkins
left a comment
derekperkins
left a comment
There was a problem hiding this comment.
These are overall good changes. I really like supporting schema/vschema application. I've also been wanting to add this examples directory with fully baked examples.
helm/vitess/templates/_vttablet.tpl
Outdated
There was a problem hiding this comment.
I think this probably needs to be implemented as another job, or maybe tacked onto the vschema job, since this will only run once. If ISM has already run, this job exits early, but people will still assume that changes to their schemas are being applied.
There was a problem hiding this comment.
Good idea. I've now created a _shard.tpl, and also moved ISM there. Let me know what you think.
helm/vitess/examples/minikube.yaml
Outdated
There was a problem hiding this comment.
Does this work with empty resources? It looks really strange. I would either expect to see resources: {} or just set some lower resources than in the default chart.
There was a problem hiding this comment.
Yeah, this is how they recommend you delete a key: https://github.com/helm/helm/blob/master/docs/chart_template_guide/values_files.md#deleting-a-default-key.
There was a problem hiding this comment.
It still feels strange. You don't want to just set low resource usage? You should be able to spin up your cluster and see how much each pod/container is consuming.
There was a problem hiding this comment.
Waiting for @dkhenry to weigh in. He's the one who recommended null resources.
In terms of resource, the concern I have is that these pods burn a lot of CPU when they come up, but then become mostly idle. Later, their load would go up depending on what the users would try. In other words, I don't know how to size these pods.
There was a problem hiding this comment.
CPU is less important since it just causes things to run slowly. It more the RAM constraints that I think are valuable, especially for someone wanting to know how big their VM needs to be for this example to come up.
I'm not against leaving null resources, but if we leave them null, it'd be helpful to have comments as to why they are null.
There was a problem hiding this comment.
@sougou For the example you can set Requests so people have an idea on how to size their VM
requests:
memory: "256Mi"
cpu: "250m"
``
That would be for the tablets and gateways, and for the control containers you can cut down the ram a bit
The default resource requirements in the helm charts are too demanding to run a small example in minikube. This change overlays a new minikube.yaml file that removes those requirements. Additionally: - Changed the NOTES to report the updated proxy URLs. - Added support for a simpler "none auth impl for mysql protocol Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
derekperkins
left a comment
derekperkins
left a comment
There was a problem hiding this comment.
Looking at this, don't we only need to apply the schema once, not once per shard?
We will still need to think about when to apply the schema. As it is currently written, helm won't delete the ApplySchema job, which means that it won't redeploy on upgrades IIRC.
- Leave it as is, so that users have to manually delete the apply schema job before it will execute again.
- Append
-{{ randAlphaNum 5 | lower }}or something similar to the job name. This will apply the schema every time because there won't be any job collisions.
I think my preference is to leave it as is and see how it goes. That requires users to be more explicit about applying schemas.
Created a separate _shard.tpl, which now contains ISM and ApplySchema. Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
sougou
left a comment
sougou
left a comment
There was a problem hiding this comment.
Still working on moving ApplySchema to _keyspace.
Also renamed k8es_secret -> secret
Spaces fixed
helm/vitess/examples/minikube.yaml
Outdated
There was a problem hiding this comment.
Waiting for @dkhenry to weigh in. He's the one who recommended null resources.
In terms of resource, the concern I have is that these pods burn a lot of CPU when they come up, but then become mostly idle. Later, their load would go up depending on what the users would try. In other words, I don't know how to size these pods.
|
As for the pod expiry, we can eventually look at using hooks: https://github.com/helm/helm/blob/master/docs/charts_hooks.md |
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
3 participants
The default resource requirements in the helm charts are too demanding
to run a small example in minikube. This change overlays a new
minikube.yaml file that removes those requirements.
Additionally: